Backend/src/ShoeStore.Application/Interface/VoucherInterface/IVoucherService.cs (1)

8-11: Remove commented-out interface members.

Keep the public service contract focused on implemented operations; track the future CRUD methods in an issue instead of commented stubs.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@Backend/src/ShoeStore.Application/Interface/VoucherInterface/IVoucherService.cs`
around lines 8 - 11, In IVoucherService remove the four commented-out interface
members (GetVoucherByGuidAsync, GetAllVouchersAsync, UpdateVoucherAsync,
DeleteVoucherAsync) so the public contract only contains implemented methods;
delete those commented lines from the IVoucherService interface file and, if you
want to track them for later, create an issue/ticket instead of leaving
commented stubs in the codebase.

Backend/src/ShoeStore.Application/DTOs/VoucherDtos/UpdateVoucherDto.cs (1)

8-14: Keep partial-update fields nullable and audit timestamps server-owned.

MaxPriceDiscount = 0, IsDeleted = false, and client-settable UpdateAt make it easy to overwrite existing data when a field is simply omitted. Set audit values in the service layer and keep optional update fields nullable.

♻️ Suggested DTO adjustment

-        public decimal MaxPriceDiscount { get; set; }
-        public DateTime UpdateAt { get; set; } = DateTime.UtcNow;
+        public decimal? MaxPriceDiscount { get; set; }
         public DateTime? ValidFrom { get; set; }
         public DateTime? ValidTo { get; set; }
         public int? MaxUsagePerUser { get; set; }
         public int? TotalQuantity { get; set; }
-        public bool? IsDeleted { get; set; } = false;
+        public bool? IsDeleted { get; set; }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Backend/src/ShoeStore.Application/DTOs/VoucherDtos/UpdateVoucherDto.cs`
around lines 8 - 14, Update the UpdateVoucherDto so optional update fields are
nullable and audit timestamps are server-owned: change MaxPriceDiscount to
decimal? (nullable) and IsDeleted to bool? (nullable) and remove the
client-settable UpdateAt (either remove the property or make it internal/handled
only by the service), and remove any default values (e.g., = false or =
DateTime.UtcNow) from the DTO; then ensure the service layer (e.g., Voucher
update handler) sets UpdateAt = DateTime.UtcNow and applies partial updates only
when nullable DTO fields (MaxPriceDiscount, IsDeleted, ValidFrom, ValidTo,
MaxUsagePerUser, TotalQuantity) are non-null.

Backend/src/ShoeStore.Application/DTOs/VoucherDtos/ResponseVoucherDto.cs (1)

7-10: Avoid shipping an empty public response DTO.

If this DTO is returned later, clients will only receive {}. Either add the voucher response fields now or remove the DTO until an endpoint actually needs it.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Backend/src/ShoeStore.Application/DTOs/VoucherDtos/ResponseVoucherDto.cs`
around lines 7 - 10, The ResponseVoucherDto class is empty; either populate it
with the actual response properties or remove it until needed. Fix by updating
the ResponseVoucherDto DTO to include the voucher response fields used by your
API (e.g., properties such as Id, Code, DiscountAmount or Percentage, ValidFrom,
ValidTo, IsActive or whatever the domain requires) so clients receive meaningful
JSON, or delete ResponseVoucherDto and refactor any controllers/services that
reference it to use the correct existing DTO/entity until you define the final
response shape.

Backend/src/ShoeStore.Application/Interface/VoucherInterface/IVoucherRepository.cs (1)

8-9: These methods are currently unused; if implemented, prefer async materialized results over IQueryable.

GetAllVouchers and GetVoucherByGuid are defined but not called anywhere in the codebase. When they're implemented, follow the established async pattern across other repositories: Task<IReadOnlyList<Voucher>> GetAllVouchersAsync(CancellationToken token) and Task<Voucher?> GetVoucherByGuidAsync(Guid voucherGuid, CancellationToken token). Returning IQueryable defers execution, cancellation, and DbContext lifetime handling to callers; materialized results provide cleaner encapsulation and align with ICategoryRepository, IColorRepository, ISizeRepository, and similar patterns in the codebase.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@Backend/src/ShoeStore.Application/Interface/VoucherInterface/IVoucherRepository.cs`
around lines 8 - 9, The interface IVoucherRepository currently exposes
IQueryable methods GetAllVouchers and GetVoucherByGuid; change these to async,
materialized signatures to match the repository pattern used elsewhere by
replacing GetAllVouchers() with Task<IReadOnlyList<Voucher>>
GetAllVouchersAsync(CancellationToken token) and GetVoucherByGuid(Guid
voucherGuid) with Task<Voucher?> GetVoucherByGuidAsync(Guid voucherGuid,
CancellationToken token), and update implementations to materialize queries with
cancellation support (e.g., ToListAsync/FirstOrDefaultAsync) so callers don't
handle IQueryable/DbContext lifetime.

Backend/src/ShoeStore.Application/Services/VoucherService.cs (2)

44-62: Remove commented-out stubs.

These commented DeleteVoucherAsync / GetAllVouchersAsync / GetVoucherByGuidAsync / UpdateVoucherAsync blocks are dead code. Track unimplemented methods via an issue/TODO rather than committed commented code; source control preserves history.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Backend/src/ShoeStore.Application/Services/VoucherService.cs` around lines 44
- 62, Remove the commented-out stub methods in VoucherService.cs (the commented
DeleteVoucherAsync, GetAllVouchersAsync, GetVoucherByGuidAsync, and
UpdateVoucherAsync blocks); delete those commented blocks entirely and, if you
need to track the missing implementations, replace them with a single TODO
comment or create a ticket/issue and reference its ID in a short TODO line
(e.g., "// TODO: implement voucher operations - ISSUE-1234") so history is
preserved by source control while keeping the codebase clean.

---

22-36: Silent defaulting of nullable required fields masks validation failures.

VoucherName ?? string.Empty, Discount ?? 0, TotalQuantity ?? 0, MinOrderPrice ?? 0 will happily create a voucher with an empty name or zero discount/quantity if validation is ever bypassed (e.g., validator not wired, or a new code path). Since CreateVoucherDto models these as nullable, the service should either reject nulls explicitly or rely on strongly-typed (non-nullable) DTO properties. Recommended: make required fields non-nullable in the DTO so the compiler/validator enforces presence and this service doesn't need to coalesce.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Backend/src/ShoeStore.Application/Services/VoucherService.cs` around lines 22
- 36, The current object initializer in VoucherService that builds the Voucher
from voucherCreateDto silently coalesces nullable fields
(voucherCreateDto.VoucherName, Discount, TotalQuantity, MinOrderPrice) to
defaults; instead either make those properties non-nullable on CreateVoucherDto
so the compiler/validation enforces them, or add explicit null/invalid checks in
VoucherService (e.g., in the method that constructs the Voucher) and throw a
clear exception (ArgumentException/ValidationException) when VoucherName is
null/empty or numeric fields are null/invalid before creating the Voucher
instance; locate the initialization in the VoucherService method that references
voucherCreateDto and update it to validate and reject nulls rather than using ??
defaults.

Backend/src/ShoeStore.Infrastructure/Repositories/VoucherRepository.cs (1)

14-17: Consider returning a single entity for GUID lookup.

GetVoucherByGuid returns IQueryable<Voucher> for what is logically a single-entity lookup (PublicId should be unique). Callers have to call FirstOrDefaultAsync themselves, and the IQueryable shape leaks EF concerns into the application layer. Consider Task<Voucher?> GetVoucherByGuidAsync(Guid voucherGuid, CancellationToken token) instead. Also, style-wise AsNoTracking() is typically applied before Where(...) (functionally equivalent here but more conventional).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Backend/src/ShoeStore.Infrastructure/Repositories/VoucherRepository.cs`
around lines 14 - 17, Change the repository method GetVoucherByGuid to return a
single entity asynchronously: replace the IQueryable-returning GetVoucherByGuid
with Task<Voucher?> GetVoucherByGuidAsync(Guid voucherGuid, CancellationToken
cancellationToken) that queries context.Vouchers.AsNoTracking().Where(v =>
v.PublicId == voucherGuid).FirstOrDefaultAsync(cancellationToken); update any
callers to await the new async method and remove any .FirstOrDefaultAsync usage
there; keep AsNoTracking() applied before Where for style/convention and ensure
you include the CancellationToken in the EF Core call.

Backend/src/ShoeStore.Api/Controllers/VoucherController.cs (1)

47-53: Created response lacks a resource location and identifier.

Created("", new { message = ... }) emits a 201 with an empty Location header and does not return the newly created voucher's identifier (e.g., PublicId). Consumers cannot discover the created resource. Consider returning the new voucher's PublicId from CreateVoucherAsync (e.g., ErrorOr<Guid> or a small result DTO) and using CreatedAtAction(nameof(GetVoucherByGuid), new { id = ... }, ...) once a GET endpoint exists, or at minimum include the PublicId in the response body.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Backend/src/ShoeStore.Api/Controllers/VoucherController.cs` around lines 47 -
53, The Created response in VoucherController currently uses Created("", ...)
which sends an empty Location header and omits the created voucher identifier;
update the CreateVoucherAsync flow (or the method returning result) to return
the new voucher PublicId (e.g., change CreateVoucherAsync to return
ErrorOr<Guid> or a DTO containing PublicId), then replace Created("", ...) with
CreatedAtAction(nameof(GetVoucherByGuid), new { id = publicId }, new { message =
"Voucher created successfully", publicId }) or at minimum include publicId in
the response body so consumers can discover the resource.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Backend/src/ShoeStore.Api/Controllers/VoucherController.cs (1)

48-69: Avoid mapping all non-404 service errors to 400.

Both endpoints collapse service failures into BadRequest, which hides conflicts or unexpected server errors and diverges from the existing controller patterns that map by ErrorType/code.

Proposed direction

         return result.Match<IActionResult>(
             _ => Created("", new { message = "Voucher created successfully" }),
-            errors => BadRequest(new
-            {
-                message = "Failed to create voucher",
-                details = errors
-            }));
+            errors => errors[0].Type switch
+            {
+                ErrorType.Conflict => Conflict(new { message = "Failed to create voucher", details = errors }),
+                ErrorType.Validation => BadRequest(new { message = "Failed to create voucher", details = errors }),
+                _ => StatusCode(StatusCodes.Status500InternalServerError,
+                    new { message = "Failed to create voucher", details = errors })
+            });

Apply the same pattern to UpdateVoucher, keeping ErrorType.NotFound => NotFound(...).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Backend/src/ShoeStore.Api/Controllers/VoucherController.cs` around lines 48 -
69, The UpdateVoucher endpoint currently maps all non-NotFound errors to
BadRequest; change the error handling for voucherService.UpdateVoucherAsync in
UpdateVoucher (and mirror CreateVoucher if applicable) to map ErrorType values
explicitly (e.g., if any error.Type == ErrorType.NotFound return NotFound(...);
if any error.Type == ErrorType.Conflict return Conflict(...); otherwise return
StatusCode(500, ...) or BadRequest depending on ErrorType) so conflicts and
server errors are not misclassified; use the existing errors collection and
ErrorType enum to decide the correct IActionResult and include error details
where appropriate.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Backend/src/ShoeStore.Api/Controllers/UserVoucherController.cs (1)

27-27: ⚠️ Potential issue | 🟡 Minor

Stale XML doc — userGuid parameter no longer exists.

The <param name="userGuid"> doc references a parameter that was removed when the GUID was sourced from the JWT claim. Drop the line so the doc matches the current signature.

-    /// <param name="userGuid">The unique identifier (GUID) of the user whose vouchers are being retrieved.</param>
     /// <param name="token">Cancellation token for the request.</param>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Backend/src/ShoeStore.Api/Controllers/UserVoucherController.cs` at line 27,
Remove the stale XML doc line that references the removed parameter "userGuid"
in the UserVoucherController XML comments so the documentation matches the
current method signature; update the docblock for the controller method that
retrieves vouchers (in class UserVoucherController, e.g., the Get.../Retrieve...
method that now reads the GUID from JWT claims) by deleting the <param
name="userGuid">...</param> entry and ensure remaining <param> tags correspond
to actual parameters.

Backend/src/ShoeStore.Application/Services/VoucherService.cs (1)

58-61: ⚠️ Potential issue | 🟡 Minor

Pass CancellationToken to FirstOrDefaultAsync.

The method receives a CancellationToken token but the database read is not cancellable. Pass the token through:

             var voucher = await voucherRepository
                 .GetVoucherByGuid(voucherGuid)
                 .Where(v => !v.IsDeleted)
-                .FirstOrDefaultAsync();
+                .FirstOrDefaultAsync(token);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Backend/src/ShoeStore.Application/Services/VoucherService.cs` around lines 58
- 61, The DB read in VoucherService is not using the provided CancellationToken;
update the call chain where
voucherRepository.GetVoucherByGuid(voucherGuid).Where(...).FirstOrDefaultAsync()
to pass the token into FirstOrDefaultAsync so the query is cancellable (keep
references to voucherRepository, GetVoucherByGuid, the Where predicate on v =>
!v.IsDeleted, and FirstOrDefaultAsync and use the method's CancellationToken
parameter named token).

Backend/src/ShoeStore.Api/Controllers/UserVoucherController.cs (1)

14-14: Stale <param> reference for userVoucherService in the class XML doc.

The class summary documents userVoucherService as a parameter, but <param> tags on a class doc don't bind to primary-constructor parameters and will not render meaningfully. Either move this description into the constructor or drop the <param> element to silence the doc warning.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Backend/src/ShoeStore.Api/Controllers/UserVoucherController.cs` at line 14,
The class XML doc for UserVoucherController contains a stale <param
name="userVoucherService"> entry which is invalid on a class; remove that
<param> tag from the class summary or relocate its description into the XML doc
for the constructor (the UserVoucherController constructor that accepts
IUserVoucherService userVoucherService) so the parameter documentation is
attached to the actual constructor parameter instead of the class.

Backend/src/ShoeStore.Infrastructure/Repositories/UserVoucherRepository.cs (1)

9-18: Add AsNoTracking() for read-only query paths.

Both methods feed into read-only projections (e.g., VoucherService.GetAllVoucherForUserAsync projects to ResponseVoucherUserDto), so EF's change tracker is unnecessary overhead. Mirroring the pattern used in UserRepository.GetAllUsers() would avoid materializing tracked entities for every query consumer:

         public IQueryable<UserVoucher> GetAllVouchers()
         {
-            return context.UserVouchers;
+            return context.UserVouchers.AsNoTracking();
         }

         public IQueryable<UserVoucher> GetVouchersByUserGuid(Guid userGuid)
         {
             return context.UserVouchers
+                .AsNoTracking()
                 .Where(uv => uv.User != null && uv.User.PublicId == userGuid);
         }

If any caller does need to track results, they can opt back in with .AsTracking().

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Backend/src/ShoeStore.Infrastructure/Repositories/UserVoucherRepository.cs`
around lines 9 - 18, The queries in GetAllVouchers and GetVouchersByUserGuid
currently return tracked entities; change them to use AsNoTracking() on
context.UserVouchers to avoid unnecessary EF change tracking for read-only
projection paths (keep the same Where predicate for GetVouchersByUserGuid) and
note callers can opt into tracking later with .AsTracking() if needed.

Backend/src/ShoeStore.Application/Interface/VoucherInterface/IVoucherService.cs (2)

11-12: Ambiguous method names — clarify intent of the two admin getters.

GetVoucherForAdminAsync and GetAllVouchersAsync have identical signatures and return types, so the difference between them is not discoverable from the API. Consider renaming to reflect intent (e.g., GetActiveVouchersForAdminAsync vs GetAllVouchersIncludingDeletedAsync) or merging into a single method with a filter parameter.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@Backend/src/ShoeStore.Application/Interface/VoucherInterface/IVoucherService.cs`
around lines 11 - 12, The two methods on IVoucherService—GetVoucherForAdminAsync
and GetAllVouchersAsync—are ambiguous because they share the same signature and
return type; either rename them to convey intent (for example,
GetActiveVouchersForAdminAsync vs GetAllVouchersIncludingDeletedAsync) or
consolidate into a single method (e.g., GetVouchersForAdminAsync) that accepts a
filter parameter/enum (e.g., includeDeleted, statusFilter) to disambiguate
behavior; update the interface method name(s) and any implementing classes and
callers (services/repositories/tests) to match the chosen approach so intent is
clear throughout the codebase.

---

19-19: Consider sourcing adminEmail from configuration rather than the call site.

Passing adminEmail as a parameter to a notification method on the service interface couples callers to admin identity. Typically the admin/sender email is a deployment concern that the service should resolve from configuration (e.g., IOptions<EmailSettings>), not something the controller/caller supplies.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@Backend/src/ShoeStore.Application/Interface/VoucherInterface/IVoucherService.cs`
at line 19, The interface method NotifyUserAboutNewVoucherAsync currently takes
adminEmail which couples callers to admin identity; change the IVoucherService
signature to remove the adminEmail parameter and instead have the concrete
implementation resolve the sender from configuration (inject
IOptions<EmailSettings> or a typed EmailSettings instance into the implementing
class), update the implementation of NotifyUserAboutNewVoucherAsync to read the
configured admin/sender email internally, and update all callers
(controllers/services) to stop passing adminEmail; also update DI registration
to ensure EmailSettings are bound and available to the service implementation.

Backend/src/ShoeStore.Infrastructure/Repositories/UserRepository.cs (1)

27-30: Unbounded GetAllUsers() for notification flow — consider scaling.

This method materializes every user when consumed by VoucherService.NotifyUserAboutNewVoucherAsync. As the user table grows, sending notifications inline on the create-voucher request thread will increasingly degrade latency and risk timeouts. Two thoughts:

1. The repo method itself is fine (it returns IQueryable, deferred), but callers should page/stream rather than enumerate the full set in memory.
2. Email-fanout for new vouchers is better suited to a background job/queue than the create endpoint.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Backend/src/ShoeStore.Infrastructure/Repositories/UserRepository.cs` around
lines 27 - 30, GetAllUsers currently returns an unbounded IQueryable that
VoucherService.NotifyUserAboutNewVoucherAsync consumes, causing full-table
enumeration during voucher creation; instead, modify the flow so the repository
exposes a paged or streaming API (e.g., Add a method like
GetUsersPaged(pageSize, pageToken) or GetUsersAsAsyncEnumerable()) and change
VoucherService.NotifyUserAboutNewVoucherAsync to enqueue a notification
job/message (with voucher id and notification metadata) to a background queue
rather than sending emails inline; implement a background worker/consumer that
pulls the job and iterates users in pages or via IAsyncEnumerable to send emails
in bounded batches with retry/backoff.

Backend/src/ShoeStore.Api/Controllers/VoucherController.cs (1)

200-256: GET / and GET /all are functionally identical — confirm intent.

GetVouchersForAdmin and GetAllVouchers both delegate to service methods that have identical implementations in VoucherService.cs (GetVoucherForAdminAsync lines 165–196 and GetAllVouchersAsync lines 131–163 are copy-pasted: same query, same projection, same filters). Either one route is intended to be paginated/filtered (and the duplication is a placeholder), or one of the two endpoints/service methods should be removed to avoid a maintenance hazard.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Backend/src/ShoeStore.Api/Controllers/VoucherController.cs` around lines 200
- 256, GetVouchersForAdmin and GetAllVouchers (and their service counterparts
GetVoucherForAdminAsync and GetAllVouchersAsync) are duplicated; confirm
intended behavior and either remove one pair or make them different: if one
should be paginated/filtered implement pagination/filter params and update
GetAllVouchersAsync (or GetVoucherForAdminAsync) to accept page/filters and
adjust the controller signature and [ProducesResponseType] docs accordingly; if
one is redundant delete the unused controller action and its service method,
update any callers/tests and OpenAPI annotations, and consolidate shared query
logic into a single private/shared method in VoucherService for reuse.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes